Conversation
Change-Id: I94ff9448ea6a2ac4089489c49d2bc61f1312a16f
This comment has been minimized.
This comment has been minimized.
Change-Id: I2f961fbc983bd29bffd2b4280021f7b696e3ad50
Change-Id: If49bdef2c665414b786ea0f2b60d0336b3839b9b
| // Only view storage has variadic buffers. | ||
| long variadicBufferLayoutCount = 0; | ||
| if (vector instanceof BaseVariableWidthViewVector) { | ||
| if (storageVector instanceof BaseVariableWidthViewVector) { |
There was a problem hiding this comment.
This now expects a variadicBufferCounts entry for extension vectors whose storage is a view vector, but c/StructVectorUnloader (which feeds this loader in Data.importIntoVectorSchemaRoot) still checks vector instanceof BaseVariableWidthViewVector on the wrapper, so it never emits one.
Importing a C Data batch with an arrow.json or OpaqueType column on Utf8View/BinaryView storage now fails with IlllegalStateException: No variadicBufferCounts available for BaseVariableWidthViewVector when all values are inlined (<= 12 bytes) or the batch is empty. The same OpaqueVector(Utf8View) case loads on main, so this is a regression for existing extension types, not only for the new one. It also affects ArrowArrayStreamReader.loadNextBatch and the dataset NativeScanner.
Could you apply the same unwrapping in StructVectorUnloader, and add a C Data round-trip test for an extension type on view storage? RoundtripTest.testExtensionTypeVector only covers UuidType.
| long variadicBufferLayoutCount = 0; | ||
| if (vector instanceof BaseVariableWidthViewVector) { | ||
| if (storageVector instanceof BaseVariableWidthViewVector) { | ||
| if (variadicBufferCounts.hasNext()) { |
There was a problem hiding this comment.
Same problem in the export direction: this now adds a variadic count for extension vectors on view storage, but c/StructVectorLoader.loadBuffers still checks the wrapper and never consumes it.
Data.exportVectorSchemaRoot (and ArrayStreamExporter) on a root with a JsonVector(Utf8View) column then throws IllegalArgumentException: not all nodes, buffers and variadicBufferCounts were consumed. It fails even with zero variadic buffers, a case that succeeds on main.
StructVectorLoader needs the same change as VectorLoader. Since all four loader/unloader classes have to agree on which vectors carry a variadic count, a single shared helper for the unwrap (for example a static storage-vector accessor on ExtensionTypeVector) would keep them in sync.
| * instance. Write UTF-8 JSON through {@link #getUnderlyingVector()}; values are not parsed or | ||
| * validated. | ||
| */ | ||
| public class JsonVector extends ExtensionTypeVector<FieldVector> |
There was a problem hiding this comment.
ExtensionTypeVector does not delegate exportCDataBuffers / getExportedCDataBufferCount to the storage vector, so JsonVector inherits the FieldVector defaults. For Utf8View storage that means Data.exportVector exports 3 buffers (validity, views, data) where ViewVarCharVector exports 4, i.e. the trailing variadic-sizes buffer is missing.
On the import side, BufferImportTypeVisitor.visitVariableWidthView takes the last buffer as the sizes buffer and computes the data buffer count as buffers.length - 3, so a consumer would misread the data buffer as sizes.
Delegating both methods to getUnderlyingVector() would fix it, ideally in ExtensionTypeVector so OpaqueVector on view storage gets it too.
3 participants
Adds
JsonTypeandJsonVectorfor the canonicalarrow.jsonextension, backed byUtf8,LargeUtf8, orUtf8View.Includes metadata handling, registration, and vector transfers that preserve the extension type. IPC loading and unloading now inspect the underlying vector when counting view buffers, so JSON backed by
Utf8Viewcan round-trip correctly.Closes #1302.